-
-
Notifications
You must be signed in to change notification settings - Fork 636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[internal] Parse addresses using a generated parser #14346
Conversation
[ci skip-build-wheels] [ci skip-rust]
[ci skip-build-wheels]
peg::parser! { | ||
grammar relative_address_parser() for str { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was really impressed with how easy this crate was to use: good choice @jsirois!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love PEG's. (first and only placed I've used/seen them is in Lua
, which I think is where they originated, from Mr Roberto I)
def99cc
to
8d5a4da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome!
Did you consider alternative locations for the Rust address
crate? Because it is not useable by any other Rust crates other than our externs stuff, it feels like it's better colocated with that all, similar to how we have fs.rs
etc.
Briefly... but I've been trying to bias away from adding more code to |
…antsbuild#14485) This issue was introduced by pantsbuild#14346 [ci skip-rust]
…antsbuild#14485) This issue was introduced by pantsbuild#14346 [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
#14347 added support for `Address` parameters, which are used via the `parametrize` builtin. But we had not yet added support for parsing parametrized addresses to CLI specs parsing. This change adds support for parsing parametrized addresses in CLI specs by generalizing the parser that was added in #14346 to support parsing the additional syntax required for CLI specs (`!` ignores and `:`/`::` wildcards), and then using the same parser (with different validation) for both `Address` and `Specs` parsing. Fixes #14521. [ci skip-build-wheels]
…sbuild#14949) pantsbuild#14347 added support for `Address` parameters, which are used via the `parametrize` builtin. But we had not yet added support for parsing parametrized addresses to CLI specs parsing. This change adds support for parsing parametrized addresses in CLI specs by generalizing the parser that was added in pantsbuild#14346 to support parsing the additional syntax required for CLI specs (`!` ignores and `:`/`::` wildcards), and then using the same parser (with different validation) for both `Address` and `Specs` parsing. Fixes pantsbuild#14521. [ci skip-build-wheels]
…rypick of #14949) (#14957) #14347 added support for `Address` parameters, which are used via the `parametrize` builtin. But we had not yet added support for parsing parametrized addresses to CLI specs parsing. This change adds support for parsing parametrized addresses in CLI specs by generalizing the parser that was added in #14346 to support parsing the additional syntax required for CLI specs (`!` ignores and `:`/`::` wildcards), and then using the same parser (with different validation) for both `Address` and `Specs` parsing. Fixes #14521. [ci skip-build-wheels]
To prepare to make the address syntax more complicated as part of #13882, move to parsing addresses using a generated
peg
parser (using the same parser crate that @jsirois introduced in #11922).Additionally, simplify
Address.spec
andAddress.path_safe_spec
slightly by removing non-trivial early returns and reducing the total number of f-strings.[ci skip-build-wheels]